Skip to content

TUI: yazi-style Miller columns, image collections, review fixes - #25

Closed
engalar wants to merge 29 commits into
mendixlabs:mainfrom
engalar:pr/tui-improvements-v2
Closed

TUI: yazi-style Miller columns, image collections, review fixes#25
engalar wants to merge 29 commits into
mendixlabs:mainfrom
engalar:pr/tui-improvements-v2

Conversation

@engalar

@engalar engalar commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • TUI rewrite: yazi-style Miller columns with tabs, async preview, borderless UI, clipboard copy, NDSL/MDL switch
  • Image support: Kitty/iTerm2/Sixel/chafa inline image rendering, ImageCollection CRUD
  • Review fixes (PR refactor(tui): yazi-style Miller columns, clipboard, debug tracing #23 feedback): fix itoa overflow, LoadTreeMsg race condition, UTF-8 truncation, PreviewEngine goroutine leaks, View() pointer receivers, dead code removal, magic number extraction, temp file cleanup
  • Package reorganization: move bson/ and tui/ into cmd/mxcli/ to colocate with CLI commands

Changes

Test plan

  • Unit tests pass (make test — 0 failures)
  • Build passes (make build)
  • go vet ./... clean
  • Manual TUI testing: navigation, tabs, preview, module role preview fallback
  • Verify image rendering in Kitty/iTerm2 terminal
  • Test with MPR v1 and v2 projects

🤖 Generated with Claude Code

engalar added 29 commits March 24, 2026 16:13
- y key copies content to clipboard in overlay and compare view
- OSC 52 terminal escape sequence for SSH/tmux environments,
  with fallback to pbcopy/wl-copy/xclip/xsel
- "✓ Copied!" flash disappears after 1 second
- Tab key switches between NDSL and MDL views in overlay
  when opened via b (BSON) or m (MDL) on a selected node
…rderless UI

Major TUI rewrite from 2-panel + overlay to yazi-inspired design:
- Miller 3-column layout (parent/current/preview) with responsive ratios
- Tab system: t/T/W/1-9/[/] for multi-location and cross-project tabs
- Async preview engine with cache, context cancellation, syntax highlighting
- Borderless minimalist style with dim separators, no box borders
- Context-sensitive key hint bar (HUD) at bottom
- Mouse support: click parent=back, current=drill in, preview=forward
- Scroll wheel in all columns including MDL/NDSL content preview
- Soft line wrapping for preview content (no truncation)
- Slide animation on column transitions (width-based, 5 frames)
- Line numbers in preview pane with scroll percentage indicator
- Clipboard copy (y) from preview content
- Banner stripping for WARNING: and Connected to: lines

New files: app.go, tab.go, miller.go, column.go, preview.go, keys.go,
tabbar.go, statusbar.go, hintbar.go, icons.go
Deleted: model.go, layout.go, panels/ (6 files)
- Column widths now adapt to content (IdealWidth with lipgloss.Width)
- Parent capped at 30%, current at 35%, preview min 25%
- Soft line wrapping in MDL/NDSL preview (wrapVisual) instead of truncation
- Scroll calculations based on visual line count after wrapping
- Preview auto-populates on drill-in/go-back (no more "No preview")
- Animation slowed to 8 frames × 50ms = 400ms, proportional shift
- Output height clamped to prevent overflow affecting other columns
- Separator rendered as exact-height column of │ characters
Writes to ~/.mxcli/tui-debug.log when MXCLI_TUI_DEBUG=1.
Traces: key/mouse events, resize, navigation, preview requests,
column width calculations, animation state.
animTickMsg was not in the forwarding list in app.go Update(),
so animation frames never decremented and parent column stayed
permanently compressed.
Width-based animation caused all text to reflow simultaneously,
creating a dizzying effect. Replaced with clean instant column
shifts matching yazi's native behavior.
- buildDescribeCmd: add systemoverview → SHOW STRUCTURE DEPTH 2
- buildDescribeCmd: return "" for virtual container nodes (security,
  category, domainmodel, navigation, projectsecurity, navprofile)
- fetch(): show "Select a document to preview" when cmd is empty
- Add TestBuildDescribeCmd coverage for systemoverview and virtual nodes

fix(sdk/mpr): resolve module names through folder hierarchy in MPR v2

In MPR v2, documents (pages, microflows, etc.) live inside Projects$Folder
units, so their ContainerID points to a folder, not the module directly.
The previous single-level moduleMap[ContainerID] lookup always missed,
causing bson dump to fail with "page not found: Module.Name".

Add resolveModuleName() which walks the container hierarchy upward until
it finds a module unit, mirroring the catalog's findModuleID logic.
Fix both GetRawUnitByName and ListRawUnits.
- Add imagecollection → Images$ImageCollection mapping in GetRawUnitByName
  and ListRawUnits (reader_units.go) so bson dump --type imagecollection works
- Add imagecollection to inferBsonType() so NDSL preview mode can dump it
- Add imagecollection to buildDescribeCmd() no-MDL-command list (returns "")
  since DESCRIBE IMAGECOLLECTION has no MDL syntax; shows friendly placeholder
- Add TDD test cases for imagecollection in TestBuildDescribeCmd (RED→GREEN)
Add executor handlers for image collection statements with module
lookup, duplicate detection, and hierarchy invalidation.
…E COLLECTION

Parse Image.Data, Format, and ID fields from BSON in parser_misc.go.
Write image data to /tmp/mxcli-preview/ and display IMAGE lines with
file paths in DESCRIBE output.
…nd security fix

- Add COLLECTION keyword to LSP completions
- Add visitor_imagecollection.go for CREATE/DROP IMAGE COLLECTION AST handling
- Fix writer_security.go to handle MaybeGeneralization in ReconcileMemberAccesses
- Add implementation plan doc
SetItems() resets cursor to 0; save and restore parentCursor/currentCursor
in navEntry so drillIn and goBack keep the selection visible.
- nanoflow: return empty (DESCRIBE NANOFLOW not implemented in executor)
- javaaction: fix syntax to DESCRIBE JAVA ACTION (two words, matching grammar)
Writer serializes image bytes under key 'Image' (Images$Image type),
but parser was looking for 'Data', so img.Data was always nil and
temp files were never written during DESCRIBE IMAGE COLLECTION.
…avaaction preview

1. drillIn(): save/restore actual item index (selectedIndex) instead of
   filtered-list cursor position so parent column highlights the correct
   item after SetItems clears the active filter.

2. fetch(): strip AS $$...$$ block from DESCRIBE JAVA ACTION output so
   the preview shows only the MDL signature, not the Java implementation.
Add 'javaaction' to inferBsonType() so Tab toggles to NDSL mode.
Add 'JavaActions$JavaAction' type prefix to GetRawUnitByName() so
bson dump can resolve javaaction by qualified name.
Normalize \r\n and \r to \n in JavaAction documentation before
rendering. Add RETURN NAME, parameter descriptions, and ICON as
MDL comments in DESCRIBE JAVA ACTION output.
Fix all critical and moderate issues from code review:
- Fix itoa() for tab numbers >= 10 (#1)
- Add TabID to LoadTreeMsg to prevent race condition (#2)
- Fix UTF-8 truncation with runewidth (#3)
- Change View()/viewZen() to pointer receivers (#4)
- Cancel PreviewEngine on tab close and app quit (#5)
- Remove ~315 lines dead code in keys.go (#6)
- Remove unused NavState, MillerFocusPreview, GoBack, ToggleZen (#7)
- Reset traceActive in CloseTrace() (#8)
- Remove nolint:govet suppression (#9)
- Extract magic numbers as named constants (#10)
- Handle openDiagram temp file cleanup and WriteString error (#11)
- Remove redundant rebuildZones() call (#12)
- Fallback NDSL preview to MDL DESCRIBE for unsupported types
- Move bson/ and tui/ packages into cmd/mxcli/ per reviewer suggestion
@engalar
engalar force-pushed the pr/tui-improvements-v2 branch from b8bdbaf to 68aaad9 Compare March 24, 2026 08:22

@ako ako left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review: TUI yazi-style Miller columns, image collections, review fixes

Alignment with Image Collection Proposal

The proposal (docs/03-development/proposals/show-describe-image-collections.md) specifies read-only SHOW/DESCRIBE. This PR goes beyond:

Proposal PR #25 Status
SHOW IMAGE COLLECTIONS [IN Module] Not implemented Missing
DESCRIBE IMAGE COLLECTION Module.Name Implemented (with temp file extraction) ✅ Extended
CREATE IMAGE COLLECTION Beyond scope
DROP IMAGE COLLECTION Beyond scope
Inline terminal image rendering (Kitty/iTerm2/Sixel/chafa) Beyond scope
Enhance parser to capture ImageFormat Yes — parser extracts Format, Data, ID

SHOW IMAGE COLLECTIONS (the simpler listing) is missing while the more complex CREATE/DROP were implemented. Consider adding it as a follow-up.

PR #23 Overlap

PR #23 was closed without merging. This PR supersedes it — all 6 commits are rebased in, plus a commit addressing all 12 review issues. No conflicts with main.

Issues to Address in Follow-ups

Moderate:

  • Image data written to /tmp/mxcli-preview/ with no cleanup (disk leak, mild security concern on shared systems)
  • detectImageProtocol prioritizes chafa over native Kitty/iTerm2 — should be reversed
  • imageFormatToExt silently defaults unknown formats to .png
  • Three plan docs (1,645 lines total) are implementation artifacts, not reference docs

Minor:

  • renderImageIterm2 reads entire file into memory with no size limit
  • Generated ANTLR parser accounts for ~12k lines of diff noise

Scope Note

This PR bundles 28 commits across 4 workstreams (TUI rewrite, image collections, package reorg, unrelated fixes). Future PRs should be scoped more narrowly for easier review.

What Looks Good

  • Image collection implementation (grammar → AST → visitor → executor → BSON writer) follows established patterns well
  • TUI image rendering with protocol auto-detection is a nice feature
  • All PR #23 review issues were addressed
  • Security fix for association member access (CE0066) is correct

Approving — the code quality is solid. Follow-up items noted above.

🤖 Generated with Claude Code

@ako

ako commented Mar 24, 2026

Copy link
Copy Markdown
Collaborator

This PR has merge conflicts with main (in mdl/grammar/MDLParser.g4 and the generated ANTLR parser files).

Could you rebase onto main? The grammar conflict is straightforward — createDemoUserStatement was added on main and needs to coexist with your createImageCollectionStatement in the createStatement rule. The generated parser files just need a make grammar regeneration after resolving.

I tested the rebase locally — build and tests pass cleanly after resolution.

@ako

ako commented Mar 24, 2026

Copy link
Copy Markdown
Collaborator

Merged manually via rebased branch push to main. All 29 commits are on main now.

@ako ako closed this Mar 24, 2026
ako pushed a commit that referenced this pull request Mar 26, 2026
Bug #25: Constants created with COMMENT 'text' were stored correctly
but DESCRIBE CONSTANT did not emit the COMMENT line. Also fixed the
CREATE CONSTANT handler to store stmt.Comment into the model's
Documentation field.
ako pushed a commit that referenced this pull request Mar 26, 2026
26 unit tests covering:
- #18: loopEndKeyword (WHILE vs LOOP), WHILE loop header formatting
- #19: Long type mapping (convertASTToMicroflowDataType, GetTypeName)
- #25: DESCRIBE CONSTANT COMMENT output, escaping, absence
- #26: Date vs DateTime type mapping and constant formatting
- #27: isQualifiedEnumLiteral, enum RETURN without $ prefix
- #28: inline if-then-else parsing and expressionToString serialization
- #23: deriveColumnName from attribute, caption, fallback, special chars

Issue #20 (XPath tokens) already covered by bugfix_test.go.
ako added a commit that referenced this pull request Mar 26, 2026
, #28)

Regenerated ANTLR parser from merged grammar to resolve conflicts
in auto-generated files.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ako pushed a commit that referenced this pull request Jul 26, 2026
Findings #25: the warm loop prints its own progress but the Mendix
runtime's own stdout/stderr — server stack traces and microflow LOG
output — went only to an in-memory buffer surfaced solely on a startup
failure. So when a page action threw, the browser showed the generic
error dialog with nothing to correlate against, and a server-side bug
couldn't be told apart from a client one.

Tee the runtime JVM's stdout+stderr to <projectDir>/.mxcli/runtime.log
via io.MultiWriter(buffer, file): the in-memory buffer still backs
startup-error reporting, and the file makes the running app debuggable
(tail -f). Appended across restarts with a "=== runtime start … ==="
marker; the handle is closed on Stop and reopened on restart. On by
default; --runtime-log <path> relocates it and "-" disables it. The path
is printed at boot.

Adds TestOpenRuntimeLog and run/run-local docs.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4
ako pushed a commit that referenced this pull request Jul 26, 2026
…tured

The runtime-log tee added earlier writes the runtime JVM's stdout/stderr to
.mxcli/runtime.log, but a standalone runtime (launched via runtimelauncher.jar)
attaches no log subscriber by default — unlike a Studio Pro / m2ee run, which
calls create_log_subscriber after start. Mendix application logs flow to log
subscribers, not stdout, so microflow LOG output and server-side exception
stack traces never reached the file: a page-action error still showed only the
generic Mendix dialog with nothing to correlate against.

After a successful start (and on every restart's Start, since each fresh JVM
starts with no subscriber), attach a "file" log subscriber pointing at the same
runtime.log with autosubscribe INFO. max_rotate is 0 so the runtime never
renames the file out from under the JVM tee's open handle; the path is passed
absolute because the runtime's cwd is <install>/runtime, not mxcli's. The
attach is best-effort — a logging failure warns to stdout rather than failing
an otherwise-healthy start.

Tests: TestStart_AttachesLogSubscriber (params), TestStart_NoLogSubscriberWhenUnset,
TestStart_LogSubscriberFailureNonFatal. Docs: run-local skill, docs-site, and the
fix-issue symptom table (findings #25, round 2).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4
ako pushed a commit that referenced this pull request Jul 26, 2026
…livers

Registering a file log subscriber (findings #25, round 2) was necessary but not
sufficient: a standalone runtime boots with the logging subsystem NOT started,
so the subscriber sits inert and receives nothing. runtime.log still held only
the JVM banner — a probe microflow's LOG output and a forced runtime exception
produced zero lines. The isolation test in the findings doc pins it exactly:
boot+subscriber → 0 probe lines; then call start_logging → 6 lines.

After create_log_subscriber, call the start_logging admin action in the same
step (order: create subscriber → start_logging). An "already started" response
is treated as success, because Start re-runs on the DB-update retry and restart
paths against a still-running JVM. Still best-effort — a logging hiccup warns to
stdout and never fails an otherwise-healthy start.

Renamed attachFileLogSubscriber → configureRuntimeLogging since it now does
both. Tests: TestStart_StartLoggingAlreadyStartedIsSuccess; TestStart_Attaches-
LogSubscriber now asserts the [start, create_log_subscriber, start_logging]
sequence.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4
ako pushed a commit that referenced this pull request Jul 27, 2026
…an filters

Completes the OpenTelemetry findings: metrics (--metrics/--runtime-setting) plus
traces (--trace).

- --trace attaches the runtime's bundled opentelemetry-javaagent (globbed at
  <runtimeDir>/agents/opentelemetry-javaagent*.jar) to the runtime JVM via
  JAVA_TOOL_OPTIONS, and defaults OTEL_SERVICE_NAME (the .mpr name, or
  --trace-service), OTEL_TRACES_EXPORTER=console, and metrics/logs exporters off —
  but does NOT override any OTEL_* the user already set, so exporting to an OTLP
  collector via your own env still works. Console spans ride the #25 runtime-log
  tee (mx.microflow.name / mx.microflow.depth).
- --trace ships default OpenTelemetry._RuntimeSpanFilters
  (CreateOrChangeVariable/Loop/Gateway/RetrieveFromCache) merged into the boot
  config, because unfiltered per-activity tracing is ~10x slower; an explicit
  --runtime-setting filter list wins.
- Boot prints where spans go (runtime log, or console when --runtime-log -).

Tests: withTraceEnv (agent appended once to existing JAVA_TOOL_OPTIONS, OTEL
defaults, user OTEL_* respected), buildRuntimeSettings trace default filters +
explicit-wins. Only the pre-existing environmental TestServeIntegration fails.
Docs (run-local skill + docs-site OpenTelemetry section incl. OTLP-collector
example) and CLAUDE.md updated.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants